feat(agent-workspace): auto-execute agent command on terminal connect#2364
feat(agent-workspace): auto-execute agent command on terminal connect#2364MarsKubeX wants to merge 2 commits into
Conversation
MarsKubeX
requested review from
benoitf and
jeffmaury
and removed request for
a team
📝 WalkthroughWalkthroughAgentWorkspaceManager now tracks workspaces where an agent command has already been injected, writes the configured command once per workspace on the first terminal prompt, and clears that state on workspace removal and disposal. Tests cover the new terminal command execution behavior. ChangesAgent command auto-execution
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Terminal
participant AgentWorkspaceManager
participant AgentRegistry
Terminal->>AgentWorkspaceManager: terminal data ("$ " prompt)
AgentWorkspaceManager->>AgentWorkspaceManager: check commandExecutedWorkspaces
alt not yet executed
AgentWorkspaceManager->>AgentRegistry: getAgent(AGENT_LABEL)
AgentRegistry-->>AgentWorkspaceManager: agent command
AgentWorkspaceManager->>Terminal: write agent command
AgentWorkspaceManager->>AgentWorkspaceManager: mark workspace as executed
else already executed
AgentWorkspaceManager-->>Terminal: forward output only
end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
jeffmaury
left a comment
jeffmaury
left a comment
There was a problem hiding this comment.
The command should be set at creation time see
Thanks for the suggestion @jeffmaury . Tried with |
bmahabirbu
left a comment
bmahabirbu
left a comment
There was a problem hiding this comment.
Gave it a whirl tested on my openclaw pr and it works!!
LGTM test wise
When switching to the workspace terminal for the first time, look up the agent's registered command from the sandbox labels and write it to the PTY once the shell is ready, removing the need for users to type it manually. Closes openkaiden#2357 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Marcel Bertagnini <mbertagn@redhat.com>
MarsKubeX
force-pushed
the
execute-agent-in-terminal
branch
from
3371f66 to
e6aa5ac
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts`:
- Around line 1210-1352: Add a regression test for the concurrent open race in
terminal agent command execution. The current “does not execute agent command on
subsequent connections” case only covers sequential opens, so it misses two
`getTerminalHandler()` calls for the same workspace before any PTY data arrives.
Extend the `terminal agent command execution` suite in
`agent-workspace-manager.spec.ts` to open the same workspace twice back-to-back,
then trigger data on both PTYs and assert the agent command is injected only
once via `pty.write` and `agentRegistry.getAgent` behavior.
In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts`:
- Around line 611-617: The `agent-workspace:terminal` handler currently lets
`this.agentRegistry.getAgent(agentId)` throw, which can abort terminal opening
entirely. Update the command-resolution path in `AgentWorkspaceManager` to catch
lookup failures around `getAgent` and degrade gracefully by leaving
`agentCommand` unset when resolution fails, so terminal launch continues even if
agent lookup breaks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 51afd7f0-3862-4422-b09f-09bca69afe97
📒 Files selected for processing (2)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: unit tests / macos-15
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: Windows
- GitHub Check: linter, formatters
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: macOS
- GitHub Check: unit tests / windows-2022
- GitHub Check: typecheck
- GitHub Check: Linux
⚠️ CI failures not shown inline (4)
GitHub Actions: fullsend / dispatch _ Route: feat(agent-workspace): auto-execute agent command on terminal connect
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1m�[0m
�[36;1mif [[ ! "$STAGE" =~ ^[a-z][a-z0-9_-]*$ ]]; then�[0m
�[36;1m echo "::error::Invalid stage name: must start with lowercase letter and contain only [a-z0-9_-]"�[0m
GitHub Actions: fullsend / dispatch _ Route: feat(agent-workspace): auto-execute agent command on terminal connect
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mif [[ -f .fullsend/config.yaml ]]; then�[0m
�[36;1m KILL_SWITCH=$(yq '.kill_switch // false' .fullsend/config.yaml)�[0m
�[36;1m if [[ "$KILL_SWITCH" == "true" ]]; then�[0m
�[36;1m echo "::error::Kill switch is active — all agent dispatch halted"�[0m
GitHub Actions: fullsend / dispatch _ Route: feat(agent-workspace): auto-execute agent command on terminal connect
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mEVENT_PAYLOAD=$(jq -c '{�[0m
�[36;1m issue: (.issue // null | if . then {number, html_url} else null end),�[0m
�[36;1m pull_request: (.pull_request // null | if . then {number, html_url,�[0m
�[36;1m head: {ref: .head.ref, sha: .head.sha, repo: {full_name: .head.repo.full_name}},�[0m
�[36;1m base: {ref: .base.ref, repo: {full_name: .base.repo.full_name}}} else null end),�[0m
�[36;1m comment: (.comment // null | if . then {body: .body[:4096]} else null end)�[0m
�[36;1m}' "$GITHUB_EVENT_PATH") || {�[0m
�[36;1m echo "::error::Failed to extract event payload from GITHUB_EVENT_PATH"�[0m
GitHub Actions: fullsend / 6_dispatch _ Route.txt: feat(agent-workspace): auto-execute agent command on terminal connect
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1m�[0m
�[36;1mif [[ ! "$STAGE" =~ ^[a-z][a-z0-9_-]*$ ]]; then�[0m
�[36;1m echo "::error::Invalid stage name: must start with lowercase letter and contain only [a-z0-9_-]"�[0m
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
/@/path aliases instead of relative paths for imports outside the current directory's module group; use relative imports only for sibling modules within the same directory
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
**/*.spec.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.spec.{ts,tsx,js,jsx}: Usetest()instead ofit()for test cases in Vitest unit tests
Usevi.mock(import('...'))for auto-mocking modules in unit tests; avoid manual mock factories when possible
Usevi.resetAllMocks()inbeforeEachhooks instead ofvi.clearAllMocks()for resetting mocks between tests
When an auto-mocked function or class method needs a real implementation, usevi.mocked(...)with the prototype pattern for class methods:vi.mocked(MyClass.prototype.myMethod).mockImplementation(...)
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts
packages/main/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/main/src/**/*.{ts,tsx}: UseipcHandle()to expose handlers in the main process with naming convention<registry-name>:<action>(e.g.,container-provider-registry:listContainers)
UseapiSender.send()to send events from main process to renderer for real-time updates
Long-running operations should useTaskManager.createTask()with title and action configuration
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
packages/{main,renderer,preload}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Container operations must include
engineIdparameter to identify the container engine
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
🧠 Learnings (3)
📚 Learning: 2026-03-09T08:47:09.657Z
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 1077
File: packages/main/src/plugin/skill/skill-manager.ts:80-109
Timestamp: 2026-03-09T08:47:09.657Z
Learning: In the kortex-hub/kortex repository, IPC handlers (via ipcHandle()) may be registered directly inside feature manager/service classes (e.g., SkillManager in packages/main/src/plugin/skill/skill-manager.ts) rather than exclusively in packages/main/src/plugin/index.ts. Treat this as an accepted design pattern for files under the plugin directory. Reviewers should not require centralization in index.ts; allow IPC registration proximity to the feature that owns the handler. When reviewing code, accept direct ipcHandle() registrations inside feature managers and ensure the pattern is consistently applied across similar feature-manager modules.
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
📚 Learning: 2026-05-12T17:14:02.153Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1850
File: packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte:66-70
Timestamp: 2026-05-12T17:14:02.153Z
Learning: When reviewing code that uses `AgentWorkspaceSummaryUI.runtime`, treat it as a required, non-null `string` per the `openkaiden/kdn-api` 0.12.0 schema. Therefore, code like `a.runtime.localeCompare(b.runtime)` is safe and should not trigger warnings about possible `undefined`/`null` values or suggestions to use nullish coalescing/optional chaining for `runtime` (unless the current local types still mark `runtime` as optional, indicating a schema/version mismatch).
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
📚 Learning: 2026-06-29T13:16:53.102Z
Learnt from: benoitf
Repo: openkaiden/kaiden PR: 2296
File: extensions/container/packages/extension/src/helper/socket-finder/_socket-finder-module.ts:28-29
Timestamp: 2026-06-29T13:16:53.102Z
Learning: When reviewing imports in openkaiden/kaiden TypeScript/JavaScript files, prefer the configured `/@/` path alias instead of relative imports that would require traversing out of the current directory/module group (i.e., paths containing `..` that cross boundaries).
Do not require alias conversion for descendant-path relative imports within the socket-finder module directory—for example, in `extensions/container/packages/extension/src/helper/socket-finder/**`, imports like `./podman/podman-version-detector` and `./podman/podman-windows-finder` are acceptable and should not be flagged.
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
🔇 Additional comments (4)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (3)
83-83: LGTM!Also applies to: 402-402, 711-711
622-630: 🎯 Functional CorrectnessUse an explicit readiness signal before writing the agent command
Injecting the command on the first PTYonDatachunk is only safe ifopenshell sandbox connectnever emits any banner/progress output before the shell prompt. There’s no local contract proving that, so this can still race if the CLI speaks first.
608-630: 🩺 Stability & AvailabilityPossible double-injection if two workspace terminals mount before the first registers a callback
commandExecutedWorkspacesis only updated from theonDatacallback, so two concurrentagent-workspace:terminalcalls for the same workspace can both passshouldExecuteCommandand each write the agent command to its own PTY.Move the reservation earlier, right after
agentCommandis resolved, to close the window between spawn and first output.packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts (1)
92-92: LGTM!
Wrap agentRegistry.getAgent in try/catch so a lookup failure degrades gracefully instead of blocking the terminal from opening. Reserve the workspace in commandExecutedWorkspaces before spawning the PTY to prevent double-injection when two terminals open concurrently. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Marcel Bertagnini <mbertagn@redhat.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (2)
402-402: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClear command execution state in every sandbox deletion path.
remove()clears the new per-workspace state, butdeleteOpenshellSandbox(name)also deletes sandboxes and leaves stale entries behind. Deleting and recreating a sandbox with the same id/name can then skip first-terminal auto-execution.Proposed fix
async deleteOpenshellSandbox(name: string): Promise<void> { const task = this.taskManager.createTask({ title: `Deleting workspace ${name}` }); task.state = 'running'; task.status = 'in-progress'; try { await this.openshellCli.deleteSandbox(name); + this.commandExecutedWorkspaces.delete(name); this.apiSender.send('agent-workspace-update'); task.status = 'success';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts` at line 402, The per-workspace command execution state is only being cleared in remove(), but deleteOpenshellSandbox(name) also removes sandboxes and can leave stale entries in commandExecutedWorkspaces. Update the sandbox deletion flow in agent-workspace-manager (especially deleteOpenshellSandbox and any shared deletion helper it uses) to also delete the matching workspace id/name from commandExecutedWorkspaces whenever a sandbox is removed, so recreating the same sandbox starts fresh.
609-625: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake command execution state an atomic reservation.
shouldExecuteCommandis computed before the awaitedgetAgent()call, so two concurrent terminal opens can both resolve a command and inject it. Also, Line 624 marks the workspace executed before the PTY write succeeds, so spawn/no-data failures can permanently suppress retry for this manager lifetime.Use a pending/reserved state, promote to executed only after
invocation.write()succeeds, and clear the reservation on lookup failure, no command, spawn failure, or terminal end before send.Also applies to: 634-637
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts` around lines 609 - 625, The command execution tracking in agent-workspace-manager is not atomic, so concurrent terminal opens can both pass shouldExecuteCommand and inject the agent command. Update the command state around agentCommand resolution in agent-workspace-manager to use a pending/reserved status, reserve before awaiting agentRegistry.getAgent(), and only promote to executed after invocation.write() succeeds. Clear the reservation on lookup failure, missing command, spawn/no-data failure, or if the terminal ends before the write completes, and apply the same state handling in the related terminal-send path referenced by the existing logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts`:
- Line 402: The per-workspace command execution state is only being cleared in
remove(), but deleteOpenshellSandbox(name) also removes sandboxes and can leave
stale entries in commandExecutedWorkspaces. Update the sandbox deletion flow in
agent-workspace-manager (especially deleteOpenshellSandbox and any shared
deletion helper it uses) to also delete the matching workspace id/name from
commandExecutedWorkspaces whenever a sandbox is removed, so recreating the same
sandbox starts fresh.
- Around line 609-625: The command execution tracking in agent-workspace-manager
is not atomic, so concurrent terminal opens can both pass shouldExecuteCommand
and inject the agent command. Update the command state around agentCommand
resolution in agent-workspace-manager to use a pending/reserved status, reserve
before awaiting agentRegistry.getAgent(), and only promote to executed after
invocation.write() succeeds. Clear the reservation on lookup failure, missing
command, spawn/no-data failure, or if the terminal ends before the write
completes, and apply the same state handling in the related terminal-send path
referenced by the existing logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 88ac915e-e38d-4f5b-add8-6102158fe07d
📒 Files selected for processing (1)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: Windows
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
/@/path aliases instead of relative paths for imports outside the current directory's module group; use relative imports only for sibling modules within the same directory
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
packages/main/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/main/src/**/*.{ts,tsx}: UseipcHandle()to expose handlers in the main process with naming convention<registry-name>:<action>(e.g.,container-provider-registry:listContainers)
UseapiSender.send()to send events from main process to renderer for real-time updates
Long-running operations should useTaskManager.createTask()with title and action configuration
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
packages/{main,renderer,preload}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Container operations must include
engineIdparameter to identify the container engine
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
🧠 Learnings (3)
📚 Learning: 2026-03-09T08:47:09.657Z
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 1077
File: packages/main/src/plugin/skill/skill-manager.ts:80-109
Timestamp: 2026-03-09T08:47:09.657Z
Learning: In the kortex-hub/kortex repository, IPC handlers (via ipcHandle()) may be registered directly inside feature manager/service classes (e.g., SkillManager in packages/main/src/plugin/skill/skill-manager.ts) rather than exclusively in packages/main/src/plugin/index.ts. Treat this as an accepted design pattern for files under the plugin directory. Reviewers should not require centralization in index.ts; allow IPC registration proximity to the feature that owns the handler. When reviewing code, accept direct ipcHandle() registrations inside feature managers and ensure the pattern is consistently applied across similar feature-manager modules.
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
📚 Learning: 2026-05-12T17:14:02.153Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1850
File: packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte:66-70
Timestamp: 2026-05-12T17:14:02.153Z
Learning: When reviewing code that uses `AgentWorkspaceSummaryUI.runtime`, treat it as a required, non-null `string` per the `openkaiden/kdn-api` 0.12.0 schema. Therefore, code like `a.runtime.localeCompare(b.runtime)` is safe and should not trigger warnings about possible `undefined`/`null` values or suggestions to use nullish coalescing/optional chaining for `runtime` (unless the current local types still mark `runtime` as optional, indicating a schema/version mismatch).
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
📚 Learning: 2026-06-29T13:16:53.102Z
Learnt from: benoitf
Repo: openkaiden/kaiden PR: 2296
File: extensions/container/packages/extension/src/helper/socket-finder/_socket-finder-module.ts:28-29
Timestamp: 2026-06-29T13:16:53.102Z
Learning: When reviewing imports in openkaiden/kaiden TypeScript/JavaScript files, prefer the configured `/@/` path alias instead of relative imports that would require traversing out of the current directory/module group (i.e., paths containing `..` that cross boundaries).
Do not require alias conversion for descendant-path relative imports within the socket-finder module directory—for example, in `extensions/container/packages/extension/src/helper/socket-finder/**`, imports like `./podman/podman-version-detector` and `./podman/podman-windows-finder` are acceptable and should not be flagged.
Applied to files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
🔇 Additional comments (1)
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (1)
718-718: LGTM!
3 participants
When switching to the workspace terminal for the first time, look up the agent's registered command from the sandbox labels and write it to the PTY once the shell is ready, removing the need for users to type it manually.
Closes #2357
Grabacion.de.pantalla.2026-07-03.a.las.13.50.40.mov